Fix for issue #11396 #11397
Fix for issue #11396 #11397
Conversation
* kernelDebugAdapter shouldn't try to convert the file path that it receives from the kernel based on the OS where VSCode is run, because it leads to files not being found on the kernel side, which in turn breaks debugging of a cell
* kernelDebugAdapter shouldn't try to convert the file path that it receives from the kernel based on the OS where VSCode is run, because it leads to files not being found on the kernel side, which in turn breaks debugging of a cell
|
|
There was a problem hiding this comment.
Thanks for the investigation and fix! I see a bunch of places around the debugger that use the path module, and they are all possibly suspicious. You haven't seen any other issues while debugging cross-OS? Also, are you sure there is no scenario where it's needed? I can't think of anything, I found this commit from the PR that originally added it, e9bfc0d, but no explanation unfortunately
Regarding the kernelDebugAdapter.ts for notebooks, I don't really see any risks: if you search for fileToCell and cellToFile, the only results are in the kernelDebugAdapter and kernelDebugAdapterBase, and they are only used as lookups. Manual testing also didn't reveal any issues for me. Now that you mentioned it I got curious, so I played around a bit with different setups and debug scenarios. It looks like there's a problem with debugging inside an interactive window kernel too. Judging by a quick glance at the error log, it seems to be something with the path convention too, but I need to dig deeper into this. I'll be getting back to you as soon as I discover something. |
|
I figured out why we have that, it's because dumpCell used to return mixed slashes, \ and / on windows, and those need to be normalized before we use them to set breakpoints. I filed this issue before realizing it was fixed this year: ipython/ipykernel#995 I think we need to keep the code because there will be plenty of users with environments that have the bug. Instead of calling the generic normalize method, we can do a simple check for whether the path looks like a windows path with a drive letter (matches If you see something weird going on with the interactive window too, this call is suspicious |
|
Ooh, I see, that ipython issue is something that I wouldn't have figured
out in a thousand years. I guess this renders my pull-request obsolote. If
you agree, I'll remove the PR and move this conversation to the comment
section of issue #11396.
In the meantime I did some preliminary investigation for the interactive
window debugger problem. I also started exactly where you pointed to, but
it didn't solve the problem entirely. After normalization is done properly,
there are no failed requests on the kernel side, but the UI still ignores
the breakpoints. Unfortunately I wasn't able to figure out why, but I saw
something interesting: the notebook uri looks like file:///c:/... Notice
that there's an extra /, making the uri translate to a file located at
/c:/..., which is clearly an invalid windows path. Any ideas on this?
Rob Lourens ***@***.***> ezt írta (időpont: 2022. szept. 21.,
Sze 5:13):
… I figured out why we have that, it's because dumpCell used to return mixed
slashes, \ and / on windows, and those need to be normalized before we use
them to set breakpoints. I filed this issue before realizing it was fixed
this year: ipython/ipykernel#995
<ipython/ipykernel#995> I think we need to keep
the code because there will be plenty of users with environments that have
the bug.
Instead of calling the generic normalize method, we can do a simple check
for whether the path looks like a windows path with a drive letter (matches
[a-z]:\\) and call the correct normalize method.
If you see something weird going on with the interactive window too, this
call is suspicious
https://github.com/microsoft/vscode-jupyter/blob/947e18f8b0252877362ea2491019b9fb2872e755/src/interactive-window/debugger/jupyter/kernelDebugAdapter.ts#L93
—
Reply to this email directly, view it on GitHub
<#11397 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AITBLK6BMXJ5CFQK3BG3FBDV7J4OPANCNFSM6AAAAAAQOLNX4U>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
Continuing in the issue |
Fixes #11396
package-lock.jsonhas been regenerated by runningnpm install(if dependencies have changed).